refactor(baseheightmap): Extract scorch rendering from BaseHeightMap into new W3DScorch - #3017
refactor(baseheightmap): Extract scorch rendering from BaseHeightMap into new W3DScorch#3017stephanmeesters wants to merge 1 commit into
Conversation
…into new W3DScorch
stephanmeesters
added
the
Refactor
|
| Filename | Overview |
|---|---|
| Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DScorch.cpp | Implements the extracted scorch lifecycle, geometry generation, and draw submission. |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScorch.h | Defines the real and no-op scorch-rendering implementations. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp | Replaces embedded scorch state and behavior with delegation to the new renderer. |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/BaseHeightMap.h | Removes embedded scorch data and exposes the delegated draw entry point. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/FlatHeightMap.cpp | Routes flat-terrain scorch drawing through BaseHeightMap. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp | Routes regular-terrain scorch drawing through BaseHeightMap. |
| Core/GameEngineDevice/CMakeLists.txt | Adds the extracted scorch implementation to the device-library sources. |
Sequence Diagram
sequenceDiagram
participant Terrain as HeightMap Render
participant Base as BaseHeightMap
participant Scorch as W3DScorch
participant Map as WorldHeightMap
participant DX8 as DX8Wrapper
Terrain->>Base: drawScorches()
Base->>Scorch: drawScorches(map)
Scorch->>Map: read terrain heights and flip state
Scorch->>Scorch: rebuild vertex/index buffers when invalidated
Scorch->>DX8: bind buffers, shader, and texture
Scorch->>DX8: Draw_Triangles()
Reviews (2): Last reviewed commit: "refactor(baseheightmap): Extract scorch ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #if DO_SCORCH | ||
| m_scorches = NEW W3DScorch; | ||
| #else | ||
| m_scorches = NEW W3DScorchDummy; | ||
| #endif |
There was a problem hiding this comment.
When the build configures DO_SCORCH=0, the unconditional #define DO_SCORCH 1 in BaseHeightMap.h replaces that value before this branch is evaluated, so the real renderer is constructed instead of W3DScorchDummy and the advertised disabled-scorch configuration is ineffective.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/BaseHeightMap.cpp
Line: 274-278
Comment:
**Disabled scorch branch stays enabled**
When the build configures `DO_SCORCH=0`, the unconditional `#define DO_SCORCH 1` in `BaseHeightMap.h` replaces that value before this branch is evaluated, so the real renderer is constructed instead of `W3DScorchDummy` and the advertised disabled-scorch configuration is ineffective.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
I think that's fine for now, DO_SCORCH is not defined in the cmake
There was a problem hiding this comment.
Fair enough. That said, #define DO_SCORCH 1 is still unconditionally present in BaseHeightMap.h (line ~72), which means the #else / W3DScorchDummy branch in the constructor is dead code right now — not just unused via CMake, but literally unreachable.
That's not a blocker, but if the intent is to wire up the toggle in the future, the #define DO_SCORCH 1 in the header would need to be removed so the value can come from the build system. You may want to leave a // TODO: comment there or just remove the define now to make the branch live. Either way, happy to drop the original comment if you're treating W3DScorchDummy as forward-looking scaffolding.
1 participant
W3DScorchDO_SCORCHhave mostly been removedDO_SCORCHbefore didn't compile but this has been fixed. Scorches are now disabled usingW3DScorchDummywhen usingDO_SCORCH=0W3DScorchDummycould be used in headless mode (not part of this PR)N.B. some code was in
DO_SCORCHthat didn't actually belong inside the macro, for exampleReal m_curImpassableSlope;